Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Hardhat Network #1043

Merged
merged 17 commits into from
Aug 7, 2021
Merged

Support for Hardhat Network #1043

merged 17 commits into from
Aug 7, 2021

Conversation

iamdefinitelyahuman
Copy link
Member

@iamdefinitelyahuman iamdefinitelyahuman commented Apr 6, 2021

What I did

Add preliminary support for Hardhat Network as a testing environment.

To test this, you must currently install the hardhat@dtt package. The required features for this integration will not merge to the main hardhat repo until after the Berlin fork, in the mean time this PR should also be considered a WIP.

To test, create a Brownie project and then install hardhat inside it:

brownie init
npm install --save-dev hardhat@dtt
touch hardhat.config.js

You can now use hardhat as a dev environment with --network hardhat or hardhat fork mode using --network hardhat-fork.

How I did it

Most of the groundwork for this was laid in previous PRs (#997, #998). The integration is made possible via a new backend module and a hardhat-specific middleware.

How to verify it

Try it! And please report any issues you encounter.

TODO

Before this merges, the test suite should be modified to target both hardhat and ganache. We also need to start use this in our daily lives and smoke out any weird edge cases or other oddities.

Closes #1024
Closes #1025
Closes #1026
Closes #1027

@BlinkyStitt
Copy link
Collaborator

Looks great! I think this covers everything I ran into.

Copy link
Collaborator

@banteg banteg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brownie/network/middlewares/hardhat.py Outdated Show resolved Hide resolved
brownie/network/rpc/hardhat.py Show resolved Hide resolved
brownie/network/rpc/hardhat.py Show resolved Hide resolved
banteg
banteg previously approved these changes Apr 7, 2021
Copy link
Collaborator

@banteg banteg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed all mentioned issues in suggestions. Looks good after that.

brownie/network/rpc/hardhat.py Show resolved Hide resolved
@banteg
Copy link
Collaborator

banteg commented Apr 7, 2021

There are still problems, cont'd in #1044

@iamdefinitelyahuman
Copy link
Member Author

Need to expose https://hardhat.org/guides/mainnet-forking.html#pinning-a-block and think about some sensible defaults for it

@iamdefinitelyahuman
Copy link
Member Author

Also lots of potentially interesting stuff here: https://hardhat.org/hardhat-network/#mining-modes

@banteg
Copy link
Collaborator

banteg commented Apr 7, 2021

Seems like one quality of life improvement would be an ability to inject Hardhat config via RPC.

@banteg
Copy link
Collaborator

banteg commented Apr 7, 2021

Pls cherry pick this too a06a35f

@PatrickAlphaC
Copy link
Contributor

PatrickAlphaC commented Apr 7, 2021

This is beautiful.

Just ran a few tests.... they are so FAST!

I did have to add the network though: brownie networks add development hardhat cmd='npx hardhat node' host=http://127.0.0.1 port=8545

Might be good to have that in the docs too.

@iamdefinitelyahuman
Copy link
Member Author

Some weird inconsistencies between ganache and hardhat with how time is handled.

Implementing #709 alongside this could help

@banteg
Copy link
Collaborator

banteg commented Apr 19, 2021

hardhat@dtt.1 additionally prefixes error messages with "Error: ", not sure if the change is intended.

A fix could look like this, but ideally we need to use error codes to not depend on parsing error messages.

message = result["error"]["message"].replace('Error: ', '')  # fix for hardhat@dtt.1

@PatrickAlphaC
Copy link
Contributor

Bump, what does this need to move forward?

@iamdefinitelyahuman
Copy link
Member Author

was waiting for hardhat official release - we got that a few days ago, now i guess the blocker is probably me? really just needs another round of testing and rechecking that everything works as expected, updates to docs... i'm on vacation at the moment tho so moving slowly.

@PatrickAlphaC
Copy link
Contributor

PatrickAlphaC commented May 26, 2021

Have an awesome vacation! Let me know how I can help here. Going to update most of the packages I worked on with hardhat support.

@pandadefi
Copy link
Contributor

How can I help?

@iamdefinitelyahuman
Copy link
Member Author

This is mostly working. The test suite fails from 3 broad issues:

  1. Hardhat returns 21,001 instead of 21,000 when estimating gas on a plain tx, so a bunch of places in brownie's test suite break due to an assumption of 21,000.
  2. Hardhat sets a default of 10,000 ether per account, which breaks assumptions around 100 ether per account (ganache's default).
  3. The lack of a global hardhat install breaks any tests that involve sub-processes launching in different folders, Mostly this affects the tests of the pytest plugin.

I think this is far enough along to launch it as a sort-of beta integration. As we use it in prod we'll find issues and shortcomings and I'm sure in a month or 2 it will look better than it does today. But for now.. lfgggggg

@iamdefinitelyahuman
Copy link
Member Author

Docs updates will come in another PR, I need to do some updates around the EIP1559 functionality as well prior to pushing out a 1.16.0 release.

@iamdefinitelyahuman iamdefinitelyahuman merged commit c723b88 into master Aug 7, 2021
@iamdefinitelyahuman iamdefinitelyahuman deleted the feat-hardhat branch August 7, 2021 14:33
@sambacha
Copy link

sambacha commented Aug 8, 2021

@iamdefinitelyahuman: 3. The lack of a global hardhat install breaks any tests that involve sub-processes launching in different folders, Mostly this affects the tests of the pytest plugin.

by global hardhat install, do you mean hh? see: https://www.npmjs.com/package/hardhat-shorthand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants